khelo/lib/ui/flow/team/scanner/scanner_view_model.dart (2)

25-26: Add input validation and documentation

The setter method would benefit from input validation and documentation about the expected ID format.

+ /// Sets the list of already processed IDs to prevent duplicate scanning
+ /// @param addedIds List of unique identifiers in UUID format
 void setData(List<String> addedIds) {
+  // Validate that all IDs are in the correct format
+  assert(addedIds.every((id) => id.isNotEmpty), 'IDs cannot be empty');
   _addedIds = addedIds;
 }

---

96-96: Document state class and consider nullable type

Consider making scannedId nullable instead of using an empty string default, and add documentation.

+/// Represents the state of the QR code scanner
 const factory ScannerState({
   Object? error,
   QRViewController? controller,
-  @Default('') String scannedId,
+  String? scannedId,
   @Default(false) bool flashOn,
   @Default(false) bool hasPermission,
 }) = _ScannerState;

khelo/lib/ui/flow/tournament/team_selection/team_selection_screen.dart (1)

22-22: Consider using package imports for better maintainability

Instead of using relative imports with multiple parent directory traversals, consider using package imports (e.g., package:khelo/ui/app_route.dart). This makes the code more maintainable and less prone to breaking when files are moved.

-import '../../../app_route.dart';
+import 'package:khelo/ui/app_route.dart';

khelo/lib/ui/flow/team/scanner/scanner_view_model.freezed.dart (1)

Line range hint 1-3: Reminder: This is generated code.

Any changes to this file should be made in the source file (scanner_view_model.dart) instead of modifying this generated file directly. The changes will be reflected here after running the Freezed code generator.

khelo/lib/ui/flow/team/detail/team_detail_screen.dart (3)

29-35: Add documentation for the new parameter.

The constructor is well-structured, but would benefit from documentation explaining the purpose and impact of showAddButton.

Add this documentation above the field:

+ /// Whether to show the "Add" button in the app bar.
+ /// When true, displays an add button that returns the team on press.
+ /// When false, shows more options button for admin/owner.
  final bool showAddButton;

---

75-81: Add loading state feedback to the Add button.

While the button is correctly disabled during loading, consider adding visual feedback to improve UX.

 SecondaryButton(
   context.l10n.common_add_title,
   enabled: !state.loading,
+  loading: state.loading,
   onPressed: () => context.pop(state.team),
 ),

---

82-88: Consider improving code organization and error handling.

1. The admin/owner check could be extracted to improve readability
2. The nested conditions could be simplified

-if (!widget.showAddButton &&
-    (state.team?.isAdminOrOwner(state.currentUserId) ?? false))
-  moreOptionButton(
-    context,
-    onPressed: () => _moreActionButton(context, notifier, state),
-  ),
+Widget? _buildActionButton(BuildContext context, TeamDetailState state) {
+  if (widget.showAddButton) {
+    return SecondaryButton(
+      context.l10n.common_add_title,
+      enabled: !state.loading,
+      loading: state.loading,
+      onPressed: () => context.pop(state.team),
+    );
+  }
+  
+  final isAdminOrOwner = state.team?.isAdminOrOwner(state.currentUserId) ?? false;
+  if (isAdminOrOwner) {
+    return moreOptionButton(
+      context,
+      onPressed: () => _moreActionButton(context, notifier, state),
+    );
+  }
+  
+  return null;
+}

Then in the build method:

-actions: [
-  if (widget.showAddButton)
-    SecondaryButton(...),
-  if (!widget.showAddButton && ...)
-    moreOptionButton(...),
-],
+actions: [
+  if (_buildActionButton(context, state) != null) _buildActionButton(context, state)!,
+],

khelo/lib/ui/flow/team/search_team/search_team_screen.dart (1)

62-77: Extract QR code scanning logic to improve maintainability.

The action button implementation would be more maintainable if extracted into a separate method.

Consider refactoring to:

+ Future<void> _handleQRScan(BuildContext context) async {
+   try {
+     context.showLoadingDialog();
+     final team = await AppRoute.scannerScreen(
+         isForTeam: true, addedIds: widget.excludedIds ?? [])
+         .push<TeamModel>(context);
+     if (context.mounted) {
+       context.hideLoadingDialog();
+       if (team != null) {
+         if (widget.excludedIds?.contains(team.id) ?? false) {
+           context.showErrorSnackBar(message: context.l10n.team_already_added);
+           return;
+         }
+         notifier.onTeamCellTap(team);
+         context.showSuccessSnackBar(message: context.l10n.team_added_successfully);
+       }
+     }
+   } catch (e) {
+     if (context.mounted) {
+       context.hideLoadingDialog();
+       context.showErrorSnackBar(message: context.l10n.qr_scan_failed);
+     }
+   }
+ }

  actionButton(
    context,
-   onPressed: () async {
-     final team = await AppRoute.scannerScreen(
-         isForTeam: true, addedIds: widget.excludedIds ?? [])
-         .push<TeamModel>(context);
-     if (context.mounted && team != null) notifier.onTeamCellTap(team);
-   },
+   onPressed: () => _handleQRScan(context),
    icon: SvgPicture.asset(
      Assets.images.icQrCode,
      colorFilter: ColorFilter.mode(
        context.colorScheme.textPrimary,
        BlendMode.srcIn,
      ),
+     semanticsLabel: context.l10n.scan_qr_code,
    ),
  ),

khelo/lib/ui/flow/team/scanner/scanner_screen.dart (1)

26-33: LGTM! Consider adding documentation.

The property rename from addedMembers to addedIds is a good change as it makes the class more generic. Consider adding documentation to explain the dual purpose of this screen (team/member scanning).

Add documentation above the class:

/// A screen that handles QR code scanning for both teams and members.
/// 
/// When [isForTeam] is true, scans team QR codes and navigates to team detail.
/// When [isForTeam] is false, scans member QR codes and navigates to user detail.
/// [addedIds] contains already added team/member IDs to prevent duplicates.

khelo/assets/locales/app_en.arb (1)

324-324: LGTM! Consider enhancing the error message for clarity.

The new localization string for duplicate team detection is well-placed and aligns with the PR's QR code team selection feature.

Consider making the message more specific to help users understand the context:

-  "add_team_already_added": "Team already added",
+  "add_team_already_added": "This team has already been added to the selection",

Configuration used: CodeRabbit UI
Review profile: CHILL

-final team = await AppRoute.scannerScreen(
+final scanResult = await AppRoute.scannerScreen(
       isForTeam: true,
       addedIds: widget.selectedTeams?.map((e) => e.id).toList() ?? [])
   .push<TeamModel>(context);
+if (scanResult != null) {
+  try {
+    final team = TeamModel.fromJson(scanResult);
+    if (context.mounted) notifier.onTeamCellTap(team);
+  } catch (e) {
+    if (context.mounted) {
+      context.showErrorSnackBar(context.l10n.invalid_team_qr_code);
+    }
+  }
+}

#!/bin/bash
# Description: Find and examine the source file for this generated code
# The source file should be named scanner_view_model.dart in the same directory

# Look for the source file
rg -l "part 'scanner_view_model.freezed.dart';" --type dart

#!/bin/bash
# Let's examine the source file to verify the property renaming
rg -A 5 -B 5 "userId|scannedId" khelo/lib/ui/flow/team/scanner/scanner_view_model.dart

#!/bin/bash
# Search for QR code scanner navigation and team selection handling
rg -l "scannerScreen|showAddButton.*true" --type dart

# Search for team selection handling in scanner screen
ast-grep --pattern 'onScanComplete($_) {
  $$$
  Navigator.pop($$$);
  $$$
}'

#!/bin/bash
# Check scanner screen implementation for team selection flow
rg -A 10 "onScanComplete" --type dart

# Check team selection screen for QR code integration
rg -A 10 "showAddButton.*true" --type dart

# Check navigation flow in app routes
rg -A 5 "scannerScreen|teamDetailScreen" khelo/lib/ui/app_route.dart

#!/bin/bash
# Description: Verify scanner screen implementation and team selection handling

# Check scanner screen implementation
echo "Checking scanner screen implementation..."
ast-grep --pattern 'class ScannerScreen extends $_ {
  $$$
}'

# Check team selection handling in notifier
echo "Checking team selection handling..."
ast-grep --pattern 'class SearchTeamViewNotifier extends $_ {
  $$$
  onTeamCellTap($_) {
    $$$
  }
  $$$
}'

#!/bin/bash
# Search for team selection handling with a broader pattern
echo "Searching for team selection handling in SearchTeamViewNotifier..."
rg "class SearchTeamViewNotifier" -A 10

# Search for onTeamCellTap implementation
echo "Searching for onTeamCellTap implementation..."
rg "onTeamCellTap" -A 3

# Check scanner screen state implementation
echo "Checking scanner screen state implementation..."
ast-grep --pattern 'class _ScannerScreenState extends $_ {
  $$$
}'

 final user =
     await AppRoute.scannerScreen(addedIds: notifier.getMemberIds())
         .push<UserModel>(context);
+if (context.mounted) {
+  if (user != null) {
+    notifier.selectUser(user);
+  } else {
+    // Show error message when scanning fails or is cancelled
+    ScaffoldMessenger.of(context).showSnackBar(
+      SnackBar(content: Text(context.l10n.qr_code_scan_failed)),
+    );
+  }
+}

#!/bin/bash
# Description: Verify the usage of showAddButton parameter in TeamDetailScreen

# Check how the showAddButton parameter is used in TeamDetailScreen
ast-grep --pattern 'class TeamDetailScreen {
  $$$
  bool showAddButton $$$
  $$$
}'

# Check for any conditional rendering based on showAddButton
rg -A 5 'showAddButton' --type dart

#!/bin/bash
# Description: Verify the impact of parameter changes

# Check for existing calls to ScannerScreen to ensure they've been updated
rg -A 5 'ScannerScreen\(' --type dart

# Check the implementation of ScannerScreen to understand how isForTeam is used
ast-grep --pattern 'class ScannerScreen {
  $$$
  bool isForTeam $$$
  $$$
}'

#!/bin/bash
# Search for all usages of scannerScreen route to verify parameter updates
rg -A 3 'scannerScreen\(' --type dart

# Search for any remaining references to addedMembers
rg 'addedMembers' --type dart

# Check ScannerScreen implementation to understand parameter usage
rg -A 10 'class ScannerScreen' --type dart

khelo/lib/ui/flow/tournament/team_selection/team_selection_screen.dart (1)

57-73: Consider enhancing error handling and user feedback.

While the implementation is functional, consider these improvements:

1. Add error handling for scanner failures
2. Show loading indicator during scanning
3. Provide feedback when attempting to scan an already selected team

Here's a suggested implementation:

 actionButton(
   context,
   onPressed: () async {
+    try {
+      context.showLoadingIndicator();
       final team = await AppRoute.scannerScreen(
         target: ScanTarget.team,
         addedIds: widget.selectedTeams?.map((e) => e.id).toList() ?? [],
       ).push<TeamModel>(context);
+      if (context.mounted) {
+        context.hideLoadingIndicator();
+        if (team != null) {
+          if (widget.selectedTeams?.any((t) => t.id == team.id) ?? false) {
+            context.showSnackBar(context.l10n.team_already_selected);
+            return;
+          }
+          notifier.onTeamCellTap(team);
+        }
+      }
+    } catch (e) {
+      if (context.mounted) {
+        context.hideLoadingIndicator();
+        context.showErrorSnackBar(context.l10n.scanner_error);
+      }
+    }
   },
   icon: SvgPicture.asset(
     Assets.images.icQrCode,
     colorFilter: ColorFilter.mode(
       context.colorScheme.textPrimary,
       BlendMode.srcIn,
     ),
   ),
 ),

khelo/lib/ui/flow/team/scanner/scanner_screen.dart (1)

Line range hint 236-359: Consider separating ScannerOverlay into its own file

The ScannerOverlay class is a complex custom painter implementation that would be better maintained in a separate file. This would:

• Improve code organization
• Make the main scanner screen more focused
• Make the overlay implementation reusable

Consider moving it to: lib/ui/components/scanner_overlay.dart

Configuration used: CodeRabbit UI
Review profile: CHILL

#!/bin/bash
# Check for ScanTarget enum and scanner screen implementation
ast-grep --pattern 'enum ScanTarget {
  $$$
  team
  $$$
}'

# Check for scanner screen route definition
ast-grep --pattern 'static Future<T?> scannerScreen<T>({
  $$$
  required ScanTarget target,
  required List<String> addedIds,
  $$$
})'

# Check for required localization strings
rg -l "team_already_selected|scanner_error" --type yaml

#!/bin/bash
# Let's try a broader search for scanner-related files and implementations

# Find scanner-related files
fd "scanner|qr" --type f

# Search for ScanTarget enum with more flexible pattern
ast-grep --pattern 'enum ScanTarget'

# Search for scanner screen route with more flexible pattern
ast-grep --pattern 'scannerScreen'

# Search for scanner-related strings in all files
rg "scanner|scan|qr" --type dart -l

# Look for route definitions
rg "AppRoute" --type dart -A 3

#!/bin/bash
# Let's check the scanner implementation and ScanTarget enum

# Find the scanner screen implementation
cat "khelo/lib/ui/flow/team/scanner/scanner_screen.dart"

# Look for ScanTarget enum definition
rg "enum ScanTarget" -A 5

# Check for scanner-related translations
fd "l10n|strings|translations" --type yaml